-
Notifications
You must be signed in to change notification settings - Fork 24.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Migrate Travis over to Circle #16354
Conversation
All tests are green but |
- run: | ||
name: Analyze Code | ||
command: | | ||
if [ -n "$CIRCLE_PR_NUMBER" ]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is re-added in @hramos PR as a separate job.
@grabbou I will spend some time tonight to see if an idea I have pans out, afterwards I will be able to give you an estimate of how long it should take to bring it to completion. |
This looks great! CC @ide in case he wants to take a look. |
xcode: "9.0" | ||
dependencies: | ||
pre: | ||
- xcrun instruments -w "iPhone 5s (10.3.1)" || true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can do this separately from this PR, but at some point we should switch over to iOS 11.0 here, and tvOS 11.0 in test-objc-tvos
. This would match what we're testing internally.
.circleci/config.yml
Outdated
| | ||
npm config set spin=false | ||
npm config set progress=false | ||
npm install --no-package-lock |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can consolidate this into a single command:
npm install --no-package-lock --no-spin --no-progress
(Using the separate npm config
commands is good when changing the preferences globally, but we're mostly running npm install
on separate images where the preference won't persist)
npm run lint | ||
npm run flow -- check | ||
- restore-cache: *restore-node-cache | ||
- run: *install-node-dependencies |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--no-package-lock
is new to npm@5, which is not used in the Node 6 and Node 4 tests. It's why I initially didn't set up a npm install alias to use everywhere. I do think it's alright to use it here, because earlier npm versions will silently ignore the flag.
Use {{ arch }} to maintain separate caches for macOS and Ubuntu, as the paths differ across OSs. This should only be required by the node_modules cache for now, but I'm updating all cache keys to prevent future breakage if we reuse these caches across OSs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hramos is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Glad we're moving away from 2 different CI providers!! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hramos is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Summary: react-native project is migrated travis to circleCI. but the document has some leavings. Only Docs Edited #16354 #16364 [ DOCS] Remove references to Travis changed Releases.md changed RNTester/RNTesterIntegrationTests/RNTesterSnapshotTests.m changed scripts/android-e2e-test.js Closes #17733 Differential Revision: D6806336 Pulled By: shergin fbshipit-source-id: 22d21995d718a89bb831f75ed2ff6074dd22c6fe
Summary: react-native project is migrated travis to circleCI. but the document has some leavings. Only Docs Edited facebook/react-native#16354 facebook/react-native#16364 [ DOCS] Remove references to Travis changed Releases.md changed RNTester/RNTesterIntegrationTests/RNTesterSnapshotTests.m changed scripts/android-e2e-test.js Closes facebook/react-native#17733 Differential Revision: D6806336 Pulled By: shergin fbshipit-source-id: 22d21995d718a89bb831f75ed2ff6074dd22c6fe
This pull request migrates Travis to Circle and pre-starts iOS simulators / tvOS ones as advised in documentation to speed up builds. It also uses Xcode 9.0 to build and test apps.
Note that podspec test is failing and it's been failing for a while on Travis as well (since podspec has been changed to use Cxx bridge by default). I've notified few folks here of that and we are looking to fix this test, but it's not related to the scope of this PR.
Also, previously, podspec tests were only runninng on master (disabled for PR builds) where I think it makes more sense to run them on PR builds as well (all in all, we want to prevent breakage before merging). That said, I've removed
if
check to make it run on all builds.Other small changes:
node_modules
properly (previously defined as restore_cache and save_cache but not referenced in following jobs)